fix(usage): preserve successful probe limits in /usage check - #4635
Conversation
647f818 to
9a0407c
Compare
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Maintainer review — reconstructed head 9a0407c
Verdict: APPROVE (Yeachan-Heo, maintainer; independent of PR author).
What this head is
The original head 647f818 became unmergeable after dev advanced to 6696988 (CHANGELOG [Unreleased] conflict only). Reconstructed by cherry-pick onto current dev: identical 3-file delta (+129/−21) — src/session/account-inventory.ts, new test/account-inventory-usage.test.ts, CHANGELOG entry — with authorship preserved (asdfqwerzxcc, original author date). Pushed with force-with-lease from 647f818. Source diff digest (canonical git diff --binary --full-index --no-ext-diff 6696988b...9a0407c0b7): 191aac363891f6616f0a01fc0ca07d500d4d9b7cccc3be70c41fe6237099da07.
Review findings (consensus run pr4635-usage-limits)
- Root cause confirmed in source: probe writes usage cache under a key embedding the resolved provider base URL; pre-fix inventory re-read used a baseUrl-less key (
"default"bucket) → limits dropped after a successful check, and cache-only/usagemissed the same bucket. - Fix is structurally sound: direct render of the (already
raw-stripped) probe report through theredactUsageReportallowlist removes the readback-key assumption; cache-only reads forwardmodelRegistry.getProviderBaseUrl(provider)(optional-chained; absent registry keeps legacy"default"key behavior byte-identical). - Redaction/privacy holds: redacted report ⊆ {provider, fetchedAt, limits[], metadata allowlist (email/accountId/account/user/projectId/orgId)}; no
raw; no credential material inAccountInventoryRow(type-level contract). - Non-blocking follow-ups recorded (not blocking this fix):
freshUsageCache15-min presentation freshness vs the cache tier's 5-min TTL (cosmetic, self-healing); thereport ⇒ okupstream invariant is correct but unencoded locally.
Bounded validation on this head
bun test packages/coding-agent/test/account-inventory-usage.test.ts— 2 pass (hermetic env; see note)bun test packages/ai/test/auth-storage-usage-cache.test.ts packages/ai/test/auth-storage-check-credentials.test.ts— 13 passbun test packages/coding-agent/test/usage-report-columns.test.ts packages/coding-agent/test/status-line-usage.test.ts— 9 passbun --cwd=packages/coding-agent run check— clean (biome 2846 files + tsc --noEmit)gjc accounts list --jsonfrom source — ok- Canonical binaries: dev-tip 6696988 →
66f481b8…, head 9a0407c →2db53013…; binary diff SHA-2565756dc5778db9eee67898053affe743c7aa87f031a6eadcebd72faa2608825c4, size delta +0.
Environment note (for the record)
On the maintainer workstation the new test fails only because ~/.bashrc exports OPENAI_API_KEY: providerSet then (correctly) adds the env-backed openai synthetic row, whose sourceHealth path isn't stubbed by the test's minimal AuthStorage. $credentialEnv intentionally reads home shell rc files (dev #4613 made dotenv parsing stricter; this is operator-env contamination, not a code defect). Not reproducible in CI (clean runner) — confirmed by env -i rerun: 2 pass.
9a0407c to
b760154
Compare
Fresh exact-head review requested — head
|
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Fresh exact-head review — APPROVE on b76015491b2546fb33e953df2f7e665e295d77d9
Reviewer: Yeachan-Heo (maintainer; independent of PR author asdfqwerzxcc). This review supersedes the dismissed approval on 9a0407c0b7 — nothing from that head was reused except verification method; every check below was re-run on this exact head.
Verification on b76015491b (base 44d7b6ee07)
- Code delta byte-identical to original commit
647f8188(git diff 647f8188 b76015491b -- packages/coding-agent/src/session/account-inventory.ts packages/coding-agent/test/account-inventory-usage.test.ts→ empty); only CHANGELOG[Unreleased]placement resolved. - Hermetic
bun test: 24 pass / 0 fail acrossaccount-inventory-usage,auth-storage-usage-cache,auth-storage-check-credentials,usage-report-columns,status-line-usage. bun --cwd=packages/coding-agent run checkclean (biome 2846 files +tsc --noEmit);git diff --checkclean.- Canonical binary diff SHA-256
ec817eb851fd5a1ae91370e9fd2b44eefe4e18c28f9e139cdff4d7f93c5bdceb(dev2c4dd412…vs head012bbb94…).
Review basis (consensus run pr4635-usage-limits, re-checked on this head)
- Root cause confirmed: probe writes usage cache keyed by resolved provider base URL; pre-fix read used the
"default"bucket → limits dropped. Fix renders theraw-stripped probe report directly through theredactUsageReportallowlist and forwardsgetProviderBaseUrlon cache-only reads. - Renderer compatibility re-verified (
usage-report.ts,accounts-cli.ts):row.usageshape unchanged; optional-chained consumers tolerate the now-populated usage. - Non-blocking follow-ups recorded elsewhere:
freshUsageCache15-min freshness label vs 5-min cache TTL (cosmetic); localreport ⇒ okguard.
APPROVE for merge to dev on CI green.
b760154 to
39c89f5
Compare
Fresh exact-head review requested — head
|
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Fresh exact-head review — APPROVE on 39c89f5f4ff612eac538fcb46c78de8b5767d832
Reviewer: Yeachan-Heo (maintainer; independent of PR author asdfqwerzxcc). Supersedes the two dismissed approvals (9a0407c0b7, b76015491b) — both targeted heads whose bases were superseded by dev merges (#4610/#4624, then #4616); nothing from them is reused. All checks below were re-run on this exact head.
Verification on 39c89f5f4f (base 7265a61c8e)
- PR code/test files byte-identical to the original author commit
647f8188(diff over those paths is empty); only CHANGELOG[Unreleased]sibling ordering resolved (no entries dropped). - No overlap with #4616 (disjoint files:
session-manager.tsvsaccount-inventory.ts). - Hermetic
bun test: 24 pass / 0 fail (account-inventory-usage,auth-storage-usage-cache,auth-storage-check-credentials,usage-report-columns,status-line-usage). changelog-history-guard12 pass;verify-gjc-state-writers --failclean;bun --cwd=packages/coding-agent run checkclean;git diff --checkclean.- Source three-dot digest
da3c71200f1de3bacf53ca795f4fcc699717466b65aaf915386a09c1ae4a959a; three-dot binary diff SHA-2560f4844426360430d18a325d67ad64d2132e8135ddc160203c4707d0e9f0865e1(dev8921e02b…vs head260fd5c1…).
Review basis (re-verified on this head)
- Root cause confirmed: the probe writes the usage cache keyed by the resolved provider base URL; the pre-fix inventory re-read used the
"default"bucket, dropping limits after a successful check and on cache-only reads. - Fix contract intact and scoped:
/usage checkrenders the successful probe report directly (through theredactUsageReportallowlist,rawalready stripped upstream); cache-only reads forwardgetProviderBaseUrl. No auth/provider routing semantics changed. - Non-blocking follow-ups recorded:
freshUsageCache15-min presentation freshness label vs the cache tier's 5-min TTL (cosmetic, self-healing); optional localreport ⇒ okguard.
APPROVE for squash-merge to dev once contract + product CI are green on this head.
Yeachan-Heo
changed the title
39c89f5 to
911668c
Compare
Signed reconstruction note — head
|
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Fresh exact-head review — APPROVE on 911668cda389f2852c5dd6651068ccb9e05885d2
Reviewer: Yeachan-Heo (maintainer; independent of PR author asdfqwerzxcc). Supersedes all prior approvals (9a0407c0b7, b76015491b, 39c89f5f4f) — each targeted a head whose base was superseded by dev merges; all dismissed, nothing reused. Every check below was re-run on this exact head.
Verification on 911668cda3 (base 27afb732b3)
- PR code/test files byte-identical to original author commit
647f8188(path-scoped diff empty); only CHANGELOG[Unreleased]sibling ordering resolved, no entries dropped. - No overlap with the 11 dev commits between
7265a61cand27afb732b3. - Hermetic
bun test: 24 pass / 0 fail (account-inventory-usage,auth-storage-usage-cache,auth-storage-check-credentials,usage-report-columns,status-line-usage). changelog-history-guard12 pass;verify-gjc-state-writers --failclean;bun --cwd=packages/coding-agent run checkclean;git diff --checkclean.- Source base...head digest
50ef86508e9d51a37adf170dd9391d3070c8d2b5e2968a33f88f060339819791; canonical binary diff SHA-256c6e978a4f45016190be0a218be74dabb49407cf705ac254d3ea4c450b198d6c2(basea3b5a852…, heada29b2341…).
Review basis (re-verified)
- Root cause confirmed: probe writes usage cache keyed by resolved provider base URL; pre-fix inventory re-read used the
"default"bucket → limits dropped after successful checks and on cache-only reads. - Fix contract intact and scoped:
/usage checkrenders the successful probe report directly (through theredactUsageReportallowlist,rawstripped upstream); cache-only reads forwardgetProviderBaseUrl. No auth/provider routing semantics changed. - Non-blocking follow-ups recorded:
freshUsageCache15-min presentation freshness label vs 5-min cache TTL (cosmetic, self-healing); optional localreport ⇒ okguard.
APPROVE for squash-merge to dev once exact-head contract + product CI are green.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
Successful credential probes returned usage limits, but the account inventory re-read them with a cache key that omitted the resolved provider base URL. Render explicit probe results directly and align cache-only reads with the provider URL so both usage command modes retain their limits. Lore-id: usage-limit-render-4634 Constraint: usage reports must remain redacted before entering presentation rows Rejected: force a second provider request | duplicates a successful probe Confidence: high Scope-risk: narrow Reversibility: clean-revert Tested: account inventory cache-key and direct probe report regression tests; coding-agent typecheck
911668c to
092a76f
Compare
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Fresh exact-head review — APPROVE on 092a76f3892a020d35c40c0c0b0d9e2dbf3ef0bb
Reviewer: Yeachan-Heo (maintainer; independent of PR author asdfqwerzxcc). Supersedes every prior approval (9a0407c0b7, b76015491b, 39c89f5f4f, 911668cda3) — all targeted heads whose bases were superseded by dev merges; all formally dismissed, nothing reused. Every check below was re-run on this exact head after the 33-commit dev advance.
Verification on 092a76f389 (base 416201eb5e)
- PR code/test files byte-identical to the original author commit
647f8188(path-scoped diff empty); only CHANGELOG[Unreleased]sibling ordering resolved, no entries dropped. - Overlap since
27afb732b3: none on PR files — the 33 landed commits (autoresearch/team-runtime/rlm restructure, python tooling, plugins, schemas, docs) do not touchaccount-inventory.tsorauth-storage.ts. - Hermetic
bun test(7 files): 47 pass / 0 fail — includesaccount-inventory-usage, both auth-storage usage suites,usage-report-columns,status-line-usage,notifications-live-stream(22),session-manager-resident-cache(3). changelog-history-guard12 pass;verify-gjc-state-writers --failclean;bun --cwd=packages/coding-agent run checkclean (biome + tsc);--versionandaccounts list --jsonsmoke ok;git diff --checkclean.- Source base...head digest
4add12eaa04e8f7b8cb68e347e70c45fa6df69db6b567c5755aee17c828adbf2; canonical binary diff SHA-2562a049fcd04d20ca852a802cd37e42222c594fe2f4586cbd110c38fc508e93930(basee0dba845…, head871e8f44…).
Review basis (re-verified on this head)
- Root cause confirmed: probe writes usage cache keyed by resolved provider base URL; pre-fix inventory re-read used the
"default"bucket → limits dropped after successful checks and on cache-only reads. - Fix contract intact and scoped:
/usage checkrenders the successful probe report directly (through theredactUsageReportallowlist,rawstripped upstream); cache-only reads forwardgetProviderBaseUrl. No auth/provider routing semantics changed. - Non-blocking follow-ups recorded:
freshUsageCache15-min presentation freshness label vs 5-min cache TTL (cosmetic, self-healing); optional localreport ⇒ okguard.
APPROVE for merge to dev once exact-head contract + product CI are green on this head.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
|
Heads-up from an adjacent fix, since your reconstruction evidence pins per-suite test counts. #4656 lands on the same Two things that touch your evidence bookkeeping, no action needed unless you re-run counts:
Separately, while tracing that: |
Terminal evidence — PR #4635 merged to dev ✅Merge commit: Issue #4634: closed with fix attribution to this PR. Pre-merge gates (all on exact head
|
Successful credential probes returned usage limits, but the account inventory re-read them with a cache key that omitted the resolved provider base URL. Render explicit probe results directly and align cache-only reads with the provider URL so both usage command modes retain their limits. Lore-id: usage-limit-render-4634 Constraint: usage reports must remain redacted before entering presentation rows Rejected: force a second provider request | duplicates a successful probe Confidence: high Scope-risk: narrow Reversibility: clean-revert Tested: account inventory cache-key and direct probe report regression tests; coding-agent typecheck (cherry picked from commit 50b032e)
3 participants
What
/usage checkrenders the successful stored-credential probe report directly, so provider limit windows (e.g.7 days: 24.00% used (76.0% left)) no longer vanish for healthy stored OAuth accounts./usagereads now pass each provider's resolved base URL into the stored-usage lookup, aligning the read key with the key the probe wrote (baseUrlis part of the usage-report cache key).packages/coding-agent/test/account-inventory-usage.test.ts).Closes #4634
Why
AuthStorage.checkCredentialswrites the usage cache under a key embedding the resolved provider base URL, but the account inventory re-read it with a baseUrl-less key that always hit the"default"bucket — so a successful probe's limits were dropped at render time. Rendering the probe report directly removes the readback-key assumption entirely, and forwarding the registry-resolved base URL fixes the cache-only mode the same way. No auth/provider routing semantics change beyond this bug fix.Rebase note
dev advanced to
416201eb5e9e50586c60747647dcc48c61d13600(33 merges since27afb732b3, incl. the autoresearch/team-runtime restructure); head911668cda3, its approval, contract success, and green CI were stale and were discarded (approval formally dismissed; nothing merged). Reconstructed by cherry-pick of the original commit647f8188onto416201eb5e: PR code and test files are byte-identical to the original commit (path-scoped diff empty); the only resolution is CHANGELOG[Unreleased]sibling ordering (all entries kept). Overlap since27afb732: none on PR files — the 33 landed commits touch autoresearch/team-runtime/rlm removal, python tooling, plugins, schemas, docs;account-inventory.tsandauth-storage.tsare untouched; only the CHANGELOG gained one sibling entry. Authorship preserved:asdfqwerzxcc <wowls7990@gmail.com>, author date 2026-08-17.Testing
On reconstructed head
092a76f3892a020d35c40c0c0b0d9e2dbf3ef0bb(base416201eb5e):bun test(7 files):account-inventory-usage(2),auth-storage-usage-cache(7),auth-storage-check-credentials(6),usage-report-columns(3),status-line-usage(6),notifications-live-stream(22),session-manager-resident-cache(3) — 47 pass / 0 failbun test ./scripts/changelog-history-guard.test.ts— 12 passbun scripts/verify-gjc-state-writers.ts --fail— clean (0 write sites outside sanctioned writer)bun --cwd=packages/coding-agent run check— clean (biome 2836 files +tsc --noEmit)--version→gjc/0.14.0;accounts list --json→ okgit diff --check 416201eb5e 092a76f389— clean416201eb5e→e0dba845…, head092a76f389→871e8f44…; canonical base...head binary diff SHA-2562a049fcd04d20ca852a802cd37e42222c594fe2f4586cbd110c38fc508e93930(size delta −4096)4add12eaa04e8f7b8cb68e347e70c45fa6df69db6b567c5755aee17c828adbf2OPENAI_API_KEY(env-backed synthetic-row path exercised against the test's minimal stub). Operator-env contamination, not a product defect — clean shells/CI unaffected (env -irerun); no product change made.GJC verdict
devbun checkpasses (coding-agent package check on reconstructed head)—
[repo owner's gaebal-gajae (clawdbot) 🦞]